fix: remediate 2026-07-13 external audit findings (lexical retrieval, privacy, upload, CSP, a11y)#582
Merged
Merged
Conversation
- Rewrite match_document_chunks_text around two GIN-indexable candidate branches (chunk tsv + title tsv fan-out, unioned by chunk id) instead of the OR-across-relations predicate that sequential-scanned every chunk (~17s warm on live; audit finding 1). Contract, scores, caps, ordering, and grants unchanged; the _v2 wrapper inherits the speedup. Parity/plan/ timing harness: scripts/sql/lexical-rpc-parity-check.sql (10 query shapes identical; no document_chunks seq scan; 8.3x faster on a 30k synthetic corpus). - Pin search_path on retrieval_owner_matches_v2 (advisor function_search_path_mutable; finding 6). - Revoke anon/authenticated future-object default privileges for role supabase_admin with guarded fallback and in-migration probe verification (finding 7); postgres-role defaults were already covered. - Scrub pre-HMAC plaintext query text in rag_queries, rag_query_misses and rag_retrieval_logs with salted irreversible placeholders; delete legacy raw-keyed rag_response_cache rows (finding 5). - Validate the three content_not_blank CHECK constraints (zero violating rows on live) so they are enforced rather than NOT VALID (finding 12). schema.sql mirrored, drift manifest regenerated from a clean scratch replay, schema/drift tests extended. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d key The standalone search shell still read the unscoped clinical-kb-recent-queries localStorage key, so a historical clinical query written by an older build could resurface for whoever used the browser next (2026-07-13 audit, finding 4). The shell now reads the owner-scoped sessionStorage key with the same owner derivation as the dashboard, and both surfaces purge the legacy unscoped keys on mount without migrating them (the original owner cannot be established). Covered by a unit suite for the shared storage module and a @critical Playwright regression that seeds the legacy value and asserts it is never rendered and gets deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signature checks only proved the first four bytes: any PK-prefixed stream passed as DOCX/XLSX and any %PDF-prefixed stream passed as PDF (2026-07-13 audit, finding 10). Uploads now must be readable ZIP packages with [Content_Types].xml and the expected main part, no macro content (vbaProject.bin or macroEnabled content types), no non-hyperlink external relationships, safe entry paths, and bounded entry count/decompressed size/compression ratio; PDFs must carry an %%EOF trailer. Existing route tests updated to structurally minimal valid PDF fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OpenAI calls are server-side only; allowing the browser to reach the provider origin needlessly widened the exfiltration surface (2026-07-13 audit, finding 12). Tests now assert the provider origin stays out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The split-pane tool cards and wide-section rows exposed only concatenated card text as their accessible name while their Link siblings carried explicit labels (2026-07-13 audit, finding 8 sub-item). Button variants now carry "Preview <tool>" labels; the collapse spec asserts the explicit name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Updates to Preview Branch (claude/audit-remediation-2026-07-13) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
BigSimmo
enabled auto-merge (squash)
July 13, 2026 12:38
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesDatabase audit hardening
Upload structure validation
Scoped recent-query storage
Browser and UI hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ClinicalDashboard.tsx (1)
1177-1201: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReimplements
loadRecentQueriesinstead of calling it.This effect duplicates the exact parse/filter/cap logic already implemented and tested in
recent-query-storage.ts'sloadRecentQueries, which was imported forglobal-search-shell.tsxbut not here. Any future change to the shared helper (cap size, filtering rule) won't propagate to this copy.♻️ Use the shared helper
-import { clearLegacyRecentQueries, demoRecentQueryOwnerId } from "`@/components/clinical-dashboard/recent-query-storage`"; +import { + clearLegacyRecentQueries, + demoRecentQueryOwnerId, + loadRecentQueries, +} from "`@/components/clinical-dashboard/recent-query-storage`";useEffect(() => { - if (!answerThreadOwnerId) { - queueMicrotask(() => setRecentQueries([])); - return; - } let cancelled = false; const frame = window.requestAnimationFrame(() => { - try { - const stored = JSON.parse( - window.sessionStorage.getItem(`${recentQueryStorageKey}:${answerThreadOwnerId}`) ?? "[]", - ); - if (Array.isArray(stored) && !cancelled) { - setRecentQueries( - stored.filter((item): item is string => typeof item === "string" && Boolean(item.trim())).slice(0, 5), - ); - } - } catch { - if (!cancelled) setRecentQueries([]); - } + if (!cancelled) setRecentQueries(loadRecentQueries(answerThreadOwnerId)); }); return () => { cancelled = true; window.cancelAnimationFrame(frame); }; }, [answerThreadOwnerId]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ClinicalDashboard.tsx` around lines 1177 - 1201, Update the recent-query-loading useEffect in ClinicalDashboard to call the shared loadRecentQueries helper from recent-query-storage.ts instead of duplicating JSON parsing, filtering, and limiting logic. Preserve the existing answerThreadOwnerId guard, animation-frame scheduling, cancellation handling, and empty-state behavior while reusing the helper’s result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/process-hardening.md`:
- Line 316: Update the “Legacy rag query text scrub (audit finding 5)”
documentation to cover all four migration checks: scrubbing rag_queries.query,
rag_queries.normalized_query, and rag_retrieval_logs, plus deletion of legacy
rag_response_cache rows. Expand the operator post-apply verification guidance
with the expected checks and outcomes for each affected table or operation.
In `@src/lib/upload-structure.ts`:
- Around line 39-45: The hasDangerousExternalRelationship function must detect
external relationships regardless of namespace prefixes or single- versus
double-quoted attributes. Replace the fragile tag regex with XML parsing, or
extend it to handle both forms, while preserving the hyperlink exemption; fail
conservatively when relationship XML cannot be safely parsed and add bypass
tests covering prefixed elements and single-quoted attributes.
- Around line 117-120: Update the PDF validation around the tail check in upload
structure parsing to require the final non-whitespace content to end with the
%%EOF marker, rather than accepting any occurrence in the tail. Preserve support
for trailing PDF whitespace, reject unfinished content where %%EOF appears
inside an object, and add a rejection test covering that case.
- Around line 94-96: Update the required-part validation around zip.file in the
upload structure flow to read/decompress the entry before accepting it. Handle
read or decompression failures by calling rejectUpload with the existing
package/part context, while preserving the current missing-entry rejection
behavior.
- Around line 55-62: Update the entry-path validation loop in the archive
handling logic to validate each JSZip entry’s original name, falling back to its
sanitized name when unavailable: use unsafeOriginalName ?? name rather than the
Object.keys(zip.files) key. Keep the existing entry-count check and rejectUpload
behavior unchanged.
In `@supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql`:
- Around line 20-51: Update the redaction checks in the migration statements for
rag_queries, rag_query_misses, rag_retrieval_logs, and rag_response_cache to
validate the complete token format defined by query-privacy.ts rather than only
the redacted-query: or redacted-cache: prefix. Reuse the exact query and cache
token shapes so only valid redacted values are preserved, while legacy
plaintext—including values sharing the prefix—is scrubbed or deleted.
In `@supabase/schema.sql`:
- Around line 4944-4948: Update the insufficient_privilege handler in the
default-privilege hardening block to preserve the warning details and then
re-throw the exception so deployment fails closed when hardening cannot be
applied. Apply the same re-throw behavior in migration
20260713102000_revoke_supabase_admin_default_privileges.sql, keeping the
existing operator follow-up message intact.
---
Outside diff comments:
In `@src/components/ClinicalDashboard.tsx`:
- Around line 1177-1201: Update the recent-query-loading useEffect in
ClinicalDashboard to call the shared loadRecentQueries helper from
recent-query-storage.ts instead of duplicating JSON parsing, filtering, and
limiting logic. Preserve the existing answerThreadOwnerId guard, animation-frame
scheduling, cancellation handling, and empty-state behavior while reusing the
helper’s result.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84038bec-2ab9-46f4-8f53-90ec0c131af6
📒 Files selected for processing (26)
docs/process-hardening.mdscripts/sql/lexical-rpc-parity-check.sqlsrc/app/api/upload/route.tssrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/global-search-shell.tsxsrc/components/clinical-dashboard/recent-query-storage.tssrc/components/tools-page-mockups/split-pane-refined-mockups.tsxsrc/components/tools-page-mockups/tools-page-mockup-page.tsxsrc/lib/security-headers.tssrc/lib/upload-structure.tssupabase/drift-manifest.jsonsupabase/migrations/20260713100000_index_friendly_lexical_retrieval.sqlsupabase/migrations/20260713101000_pin_retrieval_owner_matches_v2_search_path.sqlsupabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sqlsupabase/migrations/20260713103000_scrub_legacy_rag_query_text.sqlsupabase/migrations/20260713104000_validate_content_not_blank_constraints.sqlsupabase/schema.sqltests/api-validation-contract.test.tstests/private-access-routes.test.tstests/proxy.test.tstests/recent-query-storage.test.tstests/security-headers.test.tstests/supabase-schema.test.tstests/ui-smoke.spec.tstests/ui-tools-collapse.spec.tstests/upload-structure.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 6 file(s) based on 7 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Owner
Author
|
@codex do a deep review of this PR. So perform a code review and fix all the identified issues in the comments for the issues you find. Find and fix |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b89f915102
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
To use Codex here, create a Codex account and connect to github. |
Fixed 6 file(s) based on 7 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Owner
Author
|
@codex please fix all failing CI tests for me |
Owner
Author
|
@codex fix and address all comments with smallest possible fix and no regression |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53672e2609
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Summary
Testing
Notes
|
The auto-fix commit introduced three defects:
- 20260713102000 re-raised insufficient_privilege after the warning, turning
the deliberate hosted degradation into a hard abort of the migration chain
(schema.sql mirror too). Reverted with a comment stating the invariant, and
the schema test now rejects any bare re-raise in the guard.
- 20260713103000's strict '^redacted-query:[0-9a-f]{64}$' check did not accept
the migration's own 'redacted-query:legacy:<md5>' placeholders, so the
completion assertion aborted on the very rows it had just scrubbed. The
format check now accepts both valid shapes and stays idempotent on re-run.
- schema.sql changed without regenerating the drift manifest, failing the
sha-pinned drift-detection test; the revert restores the manifest match.
Kept: the upload-structure hardening (namespaced Relationship tags, quote
variants, unsafeOriginalName, required-part decompression check) and its new
tests, and the expanded scrub documentation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@supabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sql`:
- Line 45: Remove the unreachable duplicate raise statement from the exception
handler in the migration, and remove the matching duplicate from the
corresponding schema.sql replay block. Preserve the preceding re-raise and keep
both definitions consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15dcd02a-c2b3-4c6e-ac0b-8b6f88f1342c
📒 Files selected for processing (8)
docs/process-hardening.mdsrc/components/ClinicalDashboard.tsxsrc/lib/upload-structure.tssupabase/migrations/20260713102000_revoke_supabase_admin_default_privileges.sqlsupabase/migrations/20260713103000_scrub_legacy_rag_query_text.sqlsupabase/schema.sqltests/ui-smoke.spec.tstests/upload-structure.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/ui-smoke.spec.ts
- tests/upload-structure.test.ts
- src/components/ClinicalDashboard.tsx
- supabase/migrations/20260713103000_scrub_legacy_rag_query_text.sql
- docs/process-hardening.md
- supabase/schema.sql
- src/lib/upload-structure.ts
…ation Resolves the schema.sql conflict to the no-re-raise version: the re-raise (applied twice via the GitHub suggestion UI — the second statement was unreachable) would abort the whole live migration chain on hosted Supabase in exactly the scenario the guard is designed to degrade through, blocking the privacy scrub and constraint-validation migrations behind it. The lockdown remains probe-verified where it can apply, with the documented operator follow-up where it cannot, and the schema test now rejects a bare re-raise in this guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Heads-up on two fix-forward commits after the CodeRabbit auto-fix round:
Kept from the auto-fix: all the 🤖 Generated with Claude Code |
This was referenced Jul 13, 2026
BigSimmo
added a commit
that referenced
this pull request
Jul 13, 2026
…TTL (PT-16) (#597) * fix(privacy): clear recents at the session boundary and bound thread TTL (PT-16) Follow-up to #582's owner-scoped recents: sign-out and session expiry now clear every owner-scoped recents key (plus legacy residue) so the next person on a shared workstation cannot inherit recent clinical question text, and persisted answer threads carry a savedAt stamp that expires after 12h so raw query/answer text cannot idle indefinitely in long-lived tabs. Legacy payloads without the stamp stay loadable once and are stamped on next save. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(auth): purge private recents on passive sign-out --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remediates the code-addressable findings of the 2026-07-13 external repository audit (verified against current
mainbefore implementation; every finding re-confirmed with file/line evidence first).20260713100000–20260713104000):match_document_chunks_textaround two GIN-indexable candidate branches (chunk tsv probe + title tsv fan-out, unioned by chunk id) replacing the OR-across-relations predicate that sequential-scanned all ~70k chunks (~17s warm on live; audit finding 1). Identical contract/scores/caps/grants; the_v2wrapper inherits the speedup. Parity + plan + timing harness atscripts/sql/lexical-rpc-parity-check.sql: 10 query shapes identical to the legacy body, nodocument_chunksseq scan, 8.3× faster on a 30k synthetic corpus.search_pathonretrieval_owner_matches_v2(the one remainingfunction_search_path_mutableadvisor warning; finding 6).supabase_adminwith a guarded fallback and in-migration future-object probes (finding 7).rag_queries,rag_query_misses,rag_retrieval_logs) with salted irreversible placeholders; delete legacy raw-keyedrag_response_cacherows (finding 5). Live counts confirmed pre-apply: 230 / 10 / 84 / 0.content_not_blankconstraints (zero violating rows on live; finding 12).localStoragekey on both surfaces without migrating it (finding 4). New unit suite +@criticalPlaywright regression.%%EOF; finding 10).api.openai.comfrom CSPconnect-src(server-side-only provider calls; finding 12).Verification
npm run verify:pr-local(format, lint, typecheck, 1953 unit tests, production build + client-bundle secret scan)npm run verify:uiscope covered: full@criticalChromium gate (10/10) + targeted regression specs for every UI change; full 142-test stable regression set green on the sibling CI-gate branchnpm run eval:rag:offline(265/265)npm run eval:retrieval:quality— 36/36 PASS pre-apply baseline (content_mrr@10 = 0.9103); the DB function only changes at live apply, so the eval will be re-run immediately after the migrations are applied (must stay 36/36; per-case lexical latencies of 17–54s in the baseline are the audited defect and should collapse)db-reset-verifyreplays the migration chainnpm run check:supabase-project(targetsClinical KB Database/sjrfecxgysukkwxsowpy)Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
eval:retrieval:quality(36/36), re-run Supabase advisors (expect zero), verify scrub counts are 0,npm run check:drift, trigger the Eval Canary. Thesupabase_admindefault-privilege migration degrades to a WARNING on hosted ifpostgrescannot act for the role; the probe block reports it.storage.idx_objects_bucket_id_name_lowerindex (audit finding 12) has already been repaired live viaREINDEX; zero invalid indexes remain.🤖 Generated with Claude Code